Skip to content

CLI: Add --mount support for create and run - #41337

Draft
David Bennett (dkbennett) wants to merge 13 commits into
masterfrom
user/dkbennett/mount-v2
Draft

CLI: Add --mount support for create and run#41337
David Bennett (dkbennett) wants to merge 13 commits into
masterfrom
user/dkbennett/mount-v2

Conversation

@dkbennett

Copy link
Copy Markdown
Member

Summary of the Pull Request

Adds Docker-compatible --mount support to wslc container run and wslc container create.

The parser supports bind, named-volume, and tmpfs mounts, including Docker aliases, CSV quoting, read-only mounts, and supported tmpfs options. It produces a common typed mount model, rejects unsupported mount features explicitly, and detects duplicate destinations across --mount, --volume, and --tmpfs.

PR Checklist

  • Closes: Link to issue #xxx
  • Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
  • Tests: Added/updated if needed and all pass
  • Localization: All end user facing strings can be localized
  • Dev docs: Added/updated if needed
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Why parse --mount in WSLC?

This is consistent with how Docker CLI handles --mount, and it is necessary for the same fundamental reason. Docker CLI does not pass the raw --mount key/value string to Docker Engine. Its MountOpt parser validates the CLI grammar and converts it into structured mount.Mount objects, which are sent to the Engine through HostConfig.Mounts. The Engine API consumes typed mount configuration, not Docker CLI syntax.

WSLC must perform the equivalent parsing and translation because its backend boundary is also structured. The runtime and COM transport accept type-specific mount data, not an opaque Docker CLI string that could be forwarded for Docker Engine to interpret.

WSLC additionally has work that must happen before the Engine request can be constructed:

  • It must distinguish bind, named-volume, and tmpfs mounts so each can use the appropriate existing transport.
  • Windows bind sources must be mounted into the utility VM and rewritten to VM-visible paths.
  • Duplicate destinations must be detected before tmpfs entries are placed into a map, where one value could otherwise overwrite another before Docker can reject it.

WSLC also requires an additional capability gate. Docker CLI can represent the full mount.Mount API object, but the current WSLC transport cannot faithfully carry every Docker mount type and option. After applying Docker-compatible syntax validation, WSLC must reject unsupported features before translation. Otherwise, accepted input could lose information silently and reach the Engine with semantics different from what the user requested.

For these reasons, forwarding the fields for Docker Engine to sort out is not possible with the current architecture. Docker CLI itself does not work that way, there is no WSLC backend boundary that accepts the original --mount string, and WSLC needs the parsed values to prepare the backend request.

The common parser is intentionally scoped in two layers:

  1. Parse and validate the CLI grammar using behavior aligned with docker/cli v25.0.3.
  2. Apply a WSLC capability gate that accepts only the options the current runtime can faithfully represent and reports explicit errors for the rest.

This preserves familiar Docker CLI behavior while avoiding silent semantic loss.

The parser lives in src/windows/common and returns a transport-neutral typed mount specification containing the mount type, source, target, read-only state, and supported tmpfs settings. The CLI currently invokes it during argument validation, but it has no dependency on CLI execution types. This keeps the parsing and capability policy reusable if a future SDK or runtime API needs to accept Docker-style mount strings.

An SDK API would normally expose typed mount fields directly rather than requiring callers to construct CLI syntax. That typed API can map to the same common mount model, keeping CLI and SDK behavior aligned while allowing the text-parser call site to move into the runtime later without rewriting the parser.

The source explicitly pins the grammar to docker/cli v25.0.3 so the parsing table can be reviewed when the bundled Docker backend changes.

Implementation

  • Adds a common typed mount::Spec model that is parsed once during CLI argument validation.
  • Moves the Docker-compatible grammar into src/windows/common/MountSpecParsing.cpp and MountSpecParsing.h.
  • Defines the recognized fields and aliases in a declarative table that records each field's option family, whether its bare form is valid, and whether it is supported, unsupported, or value-dependent.
  • Routes bind and named-volume mounts through the existing volume plumbing and tmpfs mounts through the existing tmpfs plumbing.
  • Supports Docker aliases, case handling, CSV quoting, Go-compatible boolean spellings, default volume type, and tmpfs size/mode conversion.
  • Rejects unsupported mount types and options, anonymous volumes, invalid bind or volume sources, and representation limits with localized errors.
  • Normalizes and rejects duplicate destinations across all mount flag forms.
  • Adds table-driven parser coverage with 51 valid and 72 invalid mount specifications.

Validation Steps Performed

  • Full x64 Debug build.
  • WSLCCLIMountParserUnitTests: 5/5 test methods passed, exercising 123 table-driven parser cases.
  • Focused Container_Run_Mount_* end-to-end tests: 5/5 passed.

Add a Docker-style --mount option to `wslc container run` and
`wslc container create`. The flag accepts comma-separated key=value
pairs (type=bind|volume|tmpfs, source/src, target/destination/dst,
readonly/ro) and is routed into the existing volume/tmpfs plumbing.

- Parse --mount into a ParsedMount (ArgumentValidation)
- Register the Mount argument for run/create
- Wire parsed mounts into ContainerOptions (ContainerTasks)
- Add localization strings (MountArgDescription, InvalidMountError)
- Add e2e tests (tmpfs, named volume, readonly-via-inspect, invalid
  type) and update run/create help-text expectations

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 12, 2026 23:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds Docker-compatible --mount parsing and plumbing for wslc container run / wslc container create, translating validated mount specs into the existing bind/volume/tmpfs execution paths and adding unit + E2E coverage plus localized error strings.

Changes:

  • Introduces a common mount::Spec model and Docker-grammar --mount parser under src/windows/common/.
  • Wires --mount into argument validation and container option construction, including duplicate-destination rejection across --mount/--volume/--tmpfs.
  • Adds table-driven unit tests and new E2E scenarios for --mount.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/windows/wslc/WSLCCLIMountParserUnitTests.cpp Adds table-driven unit tests for --mount parsing and destination de-duplication behavior.
test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp Adds E2E coverage for --mount tmpfs/volume/readonly and failure cases.
src/windows/wslc/tasks/ContainerTasks.cpp Plumbs parsed mount specs from CLI args into ContainerOptions and validates uniqueness.
src/windows/wslc/services/ContainerService.cpp Translates mount::Spec into launcher calls for bind/volume/tmpfs.
src/windows/wslc/services/ContainerModel.h Extends ContainerOptions with Mounts and declares destination uniqueness validation.
src/windows/wslc/services/ContainerModel.cpp Reuses named-volume validation from common parser and implements duplicate-destination detection.
src/windows/wslc/commands/ContainerRunCommand.cpp Adds --mount to container run arguments.
src/windows/wslc/commands/ContainerCreateCommand.cpp Adds --mount to container create arguments.
src/windows/wslc/arguments/SpecParsing.cpp Adds a standard header include used by parsing utilities.
src/windows/wslc/arguments/ArgumentValidation.cpp Validates/parses --mount and surfaces localized user-facing errors.
src/windows/wslc/arguments/ArgumentDefinitions.h Declares the new --mount argument in the X-macro table.
src/windows/wslc/arguments/ArgumentConvertedTypes.h Adds the converted type alias mapping for parsed mount specs.
src/windows/common/MountSpecParsing.h Declares the mount grammar version, spec model, and parsing/normalization helpers.
src/windows/common/MountSpecParsing.cpp Implements Docker-compatible --mount parsing and tmpfs option formatting.
src/windows/common/CMakeLists.txt Adds the new common parser sources/headers to the build.
localization/strings/en-US/Resources.resw Adds localized strings for invalid mount syntax and duplicate mount destinations.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/windows/common/MountSpecParsing.cpp Outdated
Comment thread test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp
Comment thread src/windows/common/MountSpecParsing.cpp
Copilot AI review requested due to automatic review settings August 13, 2026 16:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/windows/common/MountSpecParsing.cpp:583

  • FormatTmpfsOptions also omits an explicitly provided tmpfs-size=0 by skipping size when the parsed value is 0. If the user passes tmpfs-size=0, that intent should be preserved and forwarded (and kept consistent with existing --tmpfs behavior, which can pass size=0).
    if (mount.TmpfsSizeBytes.has_value() && mount.TmpfsSizeBytes.value() != 0)
    {
        options.emplace_back(std::format("size={}", FormatDockerTmpfsSize(mount.TmpfsSizeBytes.value())));
    }

src/windows/common/MountSpecParsing.cpp:579

  • FormatTmpfsOptions drops an explicitly provided tmpfs-mode=0000 because it omits the mode option when the parsed value is 0. That changes user-requested semantics (and differs from --tmpfs, which forwards options verbatim), since mode=0 is a meaningful tmpfs setting.

This issue also appears on line 580 of the same file.

    if (mount.TmpfsMode.has_value() && mount.TmpfsMode.value() != 0)
    {
        options.emplace_back(std::format("mode={:o}", mount.TmpfsMode.value()));
    }

Copilot AI review requested due to automatic review settings August 13, 2026 16:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/windows/common/MountSpecParsing.cpp:14

  • New files should use the repository’s single-line copyright header format (// Copyright (C) Microsoft Corporation. All rights reserved.). This file currently uses the older block header style.
/*++

Copyright (c) Microsoft. All rights reserved.

src/windows/common/MountSpecParsing.h:14

  • New files should use the repository’s single-line copyright header format (// Copyright (C) Microsoft Corporation. All rights reserved.). This header currently uses the older block header style.
/*++

Copyright (c) Microsoft. All rights reserved.

test/windows/wslc/WSLCCLIMountParserUnitTests.cpp:14

  • New files should use the repository’s single-line copyright header format (// Copyright (C) Microsoft Corporation. All rights reserved.). This test file currently uses the older block header style.
/*++

Copyright (c) Microsoft. All rights reserved.

Copilot AI review requested due to automatic review settings August 13, 2026 19:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/windows/common/MountSpecParsing.cpp:159

  • Minor typo in the digit set passed to find_last_of: it includes an extra '0' ("01234567890. "), which is confusing to readers even though it likely doesn’t change behavior.
        const auto separator = input.find_last_of("01234567890. ");

src/windows/wslc/arguments/ArgumentValidation.cpp:234

  • The mount parser’s ValidationException::Reason() is composed of hard-coded English strings (from MountSpecParsing.cpp) and is surfaced directly to users via WSLCCLI_InvalidMountError. This means a significant portion of the user-facing error text is not localizable, which conflicts with the PR’s stated goal of localized errors for unsupported/invalid mount specs.
            catch (const mount::ValidationException& ex)
            {
                throw ArgumentException(Localization::WSLCCLI_InvalidMountError(value, ex.Reason()));
            }

Comment thread src/windows/wslc/services/ContainerService.cpp Outdated
Comment thread src/windows/common/MountSpecParsing.cpp Outdated
Comment thread src/windows/wslc/arguments/ArgumentValidation.cpp
Comment thread src/windows/wslc/services/ContainerService.cpp
Comment thread src/windows/common/MountSpecParsing.cpp Outdated
{
try
{
mount::ValidateMountCollection(options.Mounts);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mounts are validated here, then another set is allocated for dests and processed again. i think for --volume and --tmpfs, parsing happens here and in ContainerService. it would be good to convert all the flags into one mount collection and then validation, dupes, etc. can be done on that one collection

Comment thread src/windows/common/MountSpecParsing.cpp
Comment thread src/windows/wslc/arguments/ArgumentValidation.cpp
Comment thread test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp
Comment thread test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp
Comment thread test/windows/wslc/e2e/WSLCE2EContainerRunTests.cpp
L"/wslc-tmpfs/data\"",
DebianImage.NameAndTag()));
result.Verify({.Stdout = L"tmpfs_test", .Stderr = L"", .ExitCode = 0});
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should add/augment e2e tests to verify that tmpfs size/mode are applied correctly, instead of only testing that the mount is usable

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tmpfs E2E test now specifies tmpfs-size=1MB and tmpfs-mode=0700, then verifies the mounted filesystem reports a 1024 KiB capacity and mode 700 at runtime.

Comment thread src/windows/wslc/services/ContainerService.cpp
Copilot AI review requested due to automatic review settings August 17, 2026 19:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (2)

test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp:800

  • This test is in the container create suite and is named as a create test, but the command under test is container run. That makes it easy to accidentally miss a create-specific regression (and it’s inconsistent with the other newly added --mount create tests in this section).
        auto result = RunWslc(std::format(
            L"container run --name {} --mount \"type=bind,source={},target=/data\" {} true",
            WslcContainerName,
            source.wstring(),
            AlpineImage.NameAndTag()));

src/windows/common/MountSpecParsing.cpp:363

  • --mount type=bind sources are normalized to an absolute path when the user passes . or .\..., but ./... (also a common relative form on Windows in some shells) is not handled and will be rejected as non-absolute later. If relative bind sources are intended to be accepted when explicitly dot-prefixed, this should normalize ./ as well.
            mount.Source = keyValue.Value;
            if (mount.Source == L"." || mount.Source.starts_with(L".\\"))
            {
                std::error_code error;
                auto absolutePath = std::filesystem::absolute(mount.Source, error);
                if (!error)
                {
                    mount.Source = absolutePath.lexically_normal().wstring();
                }

Copilot AI review requested due to automatic review settings August 17, 2026 20:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (1)

test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp:800

  • The test name indicates container create, but the command under test uses container run. This makes the intent unclear and can hide create-vs-run behavioral differences. Consider switching this command to container create to match the test name (or rename the test if run is intentional).
        auto result = RunWslc(std::format(
            L"container run --name {} --mount \"type=bind,source={},target=/data\" {} true",
            WslcContainerName,
            source.wstring(),
            AlpineImage.NameAndTag()));

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 2ecc9835-7a6d-4f99-a077-882e6b76e02f
Copilot AI review requested due to automatic review settings August 17, 2026 20:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (5)

src/windows/wslcsession/WSLCContainer.cpp:668

  • ConvertAndValidateMounts() validates NamedVolumes[i].Name but not NamedVolumes[i].ContainerPath before passing it to addDestination(). A null ContainerPath would currently result in a generic E_INVALIDARG without actionable context. Add an explicit null check (similar to ProcessNamedVolumes).
        THROW_HR_IF_NULL_MSG(E_INVALIDARG, containerOptions.NamedVolumes[i].Name, "NamedVolume at index %lu has null Name", i);
        addDestination(containerOptions.NamedVolumes[i].ContainerPath);
    }

src/windows/wslcsession/WSLCContainer.cpp:674

  • ConvertAndValidateMounts() passes Tmpfs[i].Destination to addDestination() without validating it for null. Add a null check with the index so malformed input yields a clear diagnostic rather than a generic failure.
    for (ULONG i = 0; i < containerOptions.TmpfsCount; ++i)
    {
        addDestination(containerOptions.Tmpfs[i].Destination);
    }

src/windows/wslcsession/WSLCContainer.cpp:661

  • ConvertAndValidateMounts() checks Volumes[i].HostPath for null but passes Volumes[i].ContainerPath to addDestination() without validating it. If ContainerPath is null at this COM boundary, this will throw with an unhelpful generic error (or worse, depending on macro behavior). Add an explicit null check with the index for diagnostics.

This issue also appears in the following locations of the same file:

  • line 666
  • line 671
        THROW_HR_IF_NULL_MSG(E_INVALIDARG, containerOptions.Volumes[i].HostPath, "Volumes[%lu].HostPath is null", i);
        addDestination(containerOptions.Volumes[i].ContainerPath);

test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp:800

  • This test is named as a Container_Create scenario, but it invokes container run, which exercises a different code path (and can fail at start time rather than create time). To specifically validate container create behavior for missing bind sources, the command should use container create (and not run).
        auto result = RunWslc(std::format(
            L"container run --name {} --mount \"type=bind,source={},target=/data\" {} true",
            WslcContainerName,
            source.wstring(),
            AlpineImage.NameAndTag()));

src/windows/wslc/services/ContainerService.cpp:33

  • The namespace alias mount is introduced here but never used, which adds noise and may trigger unused-alias warnings depending on toolchain settings. It can be removed.
namespace wsl::windows::wslc::services {
namespace mount = wsl::windows::common::mount;

Copilot AI review requested due to automatic review settings August 17, 2026 21:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (4)

test/windows/wslc/WSLCCLIMountParserUnitTests.cpp:222

  • This mount spec (type=volume,...,bind-recursive=enabled) is listed as a valid case here, but the same exact input is also listed as an invalid (bind-* family mismatch) case later in c_invalidMountCases (lines ~350-351). With the current parser, bind-recursive is a bind-only option, so this entry will make the valid-case loop fail.
        {L"type=volume,source=data-volume,target=/data,bind-recursive=enabled",
         mount::Type::Volume,
         L"data-volume",
         "/data",
         false,
         {},
         {},
         ""},

test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp:802

  • This test is in WSLCE2EContainerCreateTests.cpp and is named Container_Create_*, but it invokes container run. Because run has additional behavior (and can exercise different code paths than create), this doesn’t reliably validate container create rejecting missing bind sources.
        auto result = RunWslc(std::format(
            L"container run --name {} --mount \"type=bind,source={},target=/data\" {} true",
            WslcContainerName,
            source.wstring(),
            AlpineImage.NameAndTag()));
        result.Verify({.Stdout = L"", .Stderr = FormatWslcError(Localization::MessageWslcBindSourcePathNotFound(source.wstring())), .ExitCode = 1});

test/windows/wslc/e2e/WSLCE2EContainerCreateTests.cpp:820

  • This Container_Create_* test also runs container run instead of container create, so it’s not specifically exercising the create path’s behavior around creating missing bind-source directories. Either switch to container create (and clean up the created container), or rename/move the test so it’s clear it’s validating run.
        auto result = RunWslc(std::format(
            L"container run --name {} --volume \"{}:/data\" {} true", WslcContainerName, source.wstring(), AlpineImage.NameAndTag()));
        result.Verify({.Stdout = L"", .Stderr = L"", .ExitCode = 0});
        VERIFY_IS_TRUE(std::filesystem::is_directory(source));
        EnsureContainerDoesNotExist(WslcContainerName);

src/windows/common/MountSpecParsing.cpp:585

  • PR description says anonymous volumes are rejected, but ValidateMountSpec currently allows type=volume with an empty source (i.e., anonymous volume) as long as the name is either empty or a valid named volume. Either update the PR description to match the implemented behavior, or enforce source for type=volume if anonymous volumes truly aren’t supported.
    case Type::Volume:
        if (!mount.Source.empty() && !IsValidNamedVolumeName(mount.Source))
        {
            ThrowValidation(Localization::WSLCCLI_MountVolumeSourceInvalidError());
        }

Copilot AI review requested due to automatic review settings August 17, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants